new function of clipping when generating L2 corrections, better fit used plotting L1 offsets. #14

sifuluo · 2018-02-12T18:46:49Z

No description provided.

… for L2

aperloff · 2018-02-12T22:43:37Z

JetAnalyzers/test/run_JRA_cfg.py

+        # 'root://cmsxrootd.fnal.gov//store/mc/RunIIFall17DRPremix/QCD_Pt-15to7000_TuneCP5_Flat_13TeV_pythia8/AODSIM/94X_mc2017_realistic_v10-v1/50000/00304636-1BDB-E711-B6F3-FA163ECE02A9.root',
+        # 'root://cmsxrootd.fnal.gov//store/mc/RunIIFall17DRPremix/QCD_Pt-15to7000_TuneCP5_Flat_13TeV_pythia8/AODSIM/94X_mc2017_realistic_v10-v1/50000/0036C92E-DFDB-E711-952A-008CFAFC05DE.root',
+		'root://cms-xrd-global.cern.ch//store/user/kirschen/QCD_Pt-15to7000_TuneCP5_Flat2017_13TeV_pythia8/crab_pickEvents/180201_223255/0000/pickevents_1.root'
+		# /afs/cern.ch/user/k/kirschen/public/forJERC/forMCTruthDebugging/pickevents_NoPU.root


Please remove these references to a specific file. Keep only the example file path on line 79.

sifuluo · 2018-02-13T19:12:57Z

I have already deleted that part. I am not sure if the change has taken place.

karavdin · 2018-03-16T12:26:08Z

Hi @sifuluo @aperloff,
Are there any problems with this PR, so it can't be merged? @raggleton might profit of it, he asked to do the same clipping.
@sifuluo, it is possible to make clipping flexible, i.e 8 GeV for AK4 and 17 GeV for AK8, please? At the moment we have to "fix" AK8 txt files manually.

Cheers,
Anastasia

sifuluo · 2018-03-17T03:45:08Z

The Clipping is flexible in L2 Creator. And by default, it is set to 0.0 . And In my iteration it was set to 8.0 .

FHead · 2018-04-05T11:45:07Z

Hi @sifuluo @aperloff,

What is the plan for this PR? It would be nice to have all developments in so everyone can benefit from it.

Thanks,
Yi Chen

aperloff · 2018-04-05T16:51:26Z

I'm looking at it right now. Hopefully there aren't any more issues. I have to be a little careful because I/we don't have continuous integration set up. This means that I rely heavily on the people doing the PR to test the code to make sure there are no bugs.

aperloff · 2018-04-05T16:58:42Z

JetAnalyzers/bin/compareJEC.C

-  paths.push_back(string("/home/aperloff/JEC/CMSSW_8_0_20/src/JetMETCorrections/JECDatabase/textFiles/")+cid1+"/");
-  paths.push_back(string("/home/aperloff/JEC/CMSSW_8_0_20/src/JetMETCorrections/JECDatabase/textFiles/")+cid2+"/");
-  paths.push_back(string("/home/aperloff/JEC/CMSSW_8_0_20/src/JetMETCorrections/JECDatabase/textFiles/")+cid3+"/");
+  vector<string> paths = {"","CondFormats/JetMETObjects/data/"};


It would be nice to get the person's username from either a system call or an environment variable rather than hard coding it. Also, shouldn't the paths be a command line or configuration option? I just don't like that this is hard coded. Yes, I know I didn't change it before. Be better than me.

I don't like it hard coded either, but files are not necessarily stored in /fdata/hepx/store/user//JEC// . I store different JECs are different places and they all have their own patterns of paths. So it cannot be unified, at least not on my workflow. But I made changes so that it is not just for me.

Remove the commented out lines right below this. If they aren't needed then don't leave them lying around the code.

You can follow this SO post to replace your username with the one found by the system. https://stackoverflow.com/questions/8953424/how-to-get-the-username-in-c-c-in-linux That way it will work for whomever is using the code. Also, you can have a command line option to override the search locations, but only if the user specifies something on the command line.

I don't think it a good idea to have any preset convention of JEC paths, I sometimes compare my JECs with Andrew's, fixing username in a path does not seem to be useful for myself.

Right now you have your username (and mine) hard coded in the the code. Either remove those lines completely or make it configurable. Also, the disk path (/fdata/hepx/) doesn't need to be hard coded. You can make it a command line option with a default. There are so many better ways of doing this than carrying along these hard coded paths.

aperloff · 2018-04-05T19:19:44Z

JetAnalyzers/bin/compareJEC.C

-
-  compareJEC("Summer16_25nsV5", "Summer16_25nsV4", "Spring16_25nsV6", "AK8PFPuppi", "AK8PFPuppi", "AK8PFPuppi", "MC","MC","MC",    false, true,  false);
-  compareJEC("Summer16_25nsV5", "Summer16_25nsV4", "Spring16_25nsV6", "AK8PFPuppi", "AK8PFPuppi", "AK8PFPuppi", "MC","MC","MC",    true, false,  false);
+  // compareJEC("Summer16_25nsV5", "Summer16_25nsV4", "Spring16_25nsV6", "AK8PFchs", "AK8PFchs", "AK8PFchs", "MC","MC","MC",    false, true,  false);


Take out these commented lines if you're no longer using them.

aperloff · 2018-04-05T19:21:02Z

JetAnalyzers/bin/jet_synchtest_x.cc

@@ -795,6 +796,7 @@ void MatchEventsAndJets::LoopOverEvents(bool verbose, bool reduceHistograms, str
   for (IT::const_iterator it = mapTreePU.begin(); it != mapTreePU.end(); ++it) {

      if (iftest && nevs >= maxEvts) return;
+      // if (nevs >= 10000000) return;


Remove this line

aperloff · 2018-04-05T19:21:17Z

JetAnalyzers/bin/jet_synchtest_x.cc

@@ -795,6 +796,7 @@ void MatchEventsAndJets::LoopOverEvents(bool verbose, bool reduceHistograms, str
   for (IT::const_iterator it = mapTreePU.begin(); it != mapTreePU.end(); ++it) {

      if (iftest && nevs >= maxEvts) return;
+      // if (nevs >= 10000000) return;

      //if (nevs%10000==0) cout << "\t"<<nevs << endl;


Remove this line too.

aperloff · 2018-04-05T19:27:04Z

JetUtilities/interface/ClosureMaker.hh

@@ -83,27 +83,28 @@ public:
 	void 				checkResponse();
 	pair<double,double> determineCanvasRange(double xmin, double xmax);
 	void 				makeCanvases();
-	void 				makeMergedCanvas();
+	void 				makeMergedCanvas(bool finemerge);


Any way to fix the spacing here? I know it's not your fault, but this would be a good opportunity to standardize the use of spaces and tabs.

I am converting all tabs to spaces when I modify a file now.

aperloff · 2018-04-05T19:30:11Z

JetUtilities/interface/L2Creator.hh

@@ -99,7 +99,7 @@ private:
    TString                   output, outputDir, l2calofit, l2pffit;
    vector<string>            formats, algs;
    bool                      l2l3, delphes;
-    int                       maxFitIter;
+    int                       maxFitIter, statTh;


It wasn't immediately clear to me from the name what this variable was. Can you call it "statThreshold"?

aperloff · 2018-04-05T19:33:32Z

JetUtilities/src/ClosureMaker.cc

-            hClosure.back()->SetBinContent(ibin+1,h.back()->GetMean());
-            hClosure.back()->SetBinError(ibin+1,h.back()->GetMeanError());
-        }
+        // else if(h.back()->GetEntries()<=4 && h.back()->GetEntries()>1) {


Why remove this?

There was a point in the closure whose response plot had exactly 4 entries, and it was not removed by statThreshold option. So I removed this line

Well yeah, that's the logic set up there. What do you want to have happen?

If there are more than statThreshold entries and things proceed as normal

The part you commented out. If there is less than statThreshold entries, but it's not empty, do you want some fallback behavior? That was what these lines were for. If you don't want that behavior don't just comment these lines out; remove them entirely.

else skip this point

If it does not exceed the statThreshold, I think it'd be good to skip the point to show that the correction is performing well. So I'll skip this point

aperloff · 2018-04-05T19:35:31Z

JetUtilities/src/ClosureMaker.cc

@@ -533,10 +534,10 @@ void ClosureMaker::makeCanvases() {
 }

 //______________________________________________________________________________
-void ClosureMaker::makeMergedCanvas() {
+void ClosureMaker::makeMergedCanvas(bool finemerge = false) {


I don't love this finemerge variable. To me it just seems like a way to force the canvas range to be narrower. Can't you just change the settings in the function "determineCanvasRange"? Make that function smarter rather than coming up with a way to circumvent it.

You are right, it is forcing the canvas to be narrowed to a range that high pT range should locate in. And also this is the plot people always want to see to make sure high pT region is fine. Same canvas range also makes it easier to compare between iterations.
There is such a determineCanvasRange function if I did not remember wrong. But when the closure it not good, it will try to incorporate all points in the range. making the range really large. This is of course useful to see the entire picture. So the finemerge is just in addition to this.

Why not make it so that if the dynamic range of the plot is too large it duplicates the plot and makes a "full range" version and a "zoomed in" version? Might be hard, but would make more sense from an ease of use and automation standpoint.

It is hard to define 'too large'. Actually I think it would be best that we always have such 1.05-0.95 range canvas. So the too large would be defined as 'xmin<0.95 || xmax > 1.05'. And these days, it is almost always the case.

Okay, then change the code to do that.

aperloff · 2018-04-05T19:37:46Z

JetUtilities/src/L2Creator.cc

@@ -1373,16 +1378,17 @@ void L2Creator::writeTextFileForCurrentAlgorithm_spline() {

                //For expediency of Summer16_25nsV5_MC do eta-dependent clipping
                fout<<setw(8) <<etamin<<setw(8)<<etamax
-                    <<setw(10)<<setprecision(6)<<(isection ? bounds.first : 0.001)
+                    <<setw(10)<<setprecision(6)<<(firstline ? 0.001 : bounds.first)


Did you test this? Just want to make sure the logic is right.

Yes, the new corrections are all done by this

Okay, then move onto the other comments.

aperloff · 2018-04-05T19:38:52Z

JetUtilities/src/SynchFittingProcedure.hh

         //cout << cname << "sfsg1\tnb=" << nb << endl;
+         TF1* fr = HistUtil::fit_gaussian(aux, 1.5, 1.0, 10, false);


I love this. Thank you!

sifuluo · 2018-07-02T21:03:12Z

The commented changed are made and these pieces of code is being used in Fall17 that proved its validity.

aperloff · 2018-07-05T14:02:12Z

@sifuluo There are plenty of comments that you haven't addressed. There are several about removing commented out lines. There's the comment about 'statThreshold'. There's the comment about getting the username from the environment variables. Please have a look. As soon as the comments are addressed I will merge the PR.

sifuluo · 2018-07-09T18:16:26Z

I have replied to your comments, please check them out and inform me if there's anything else needed to be addressed.

aperloff · 2018-08-07T15:36:13Z

@sifuluo Could this PR be split into multiple atomic PRs? I'd like to finish this so that I can get it off my plate, but at this point the PR is really hard to follow and covers a huge amount of code.

… dummyl3 files upon request.

…o specify ptclip value for specific cone sizes.

sifuluo added 3 commits February 2, 2018 14:51

clip at 8 for low pt

4b79df2

Adding functions of clipping pt in lower pt and statistical threshold…

d960cb0

… for L2

Fit included in HistogramUtility

691f9be

aperloff reviewed Feb 12, 2018

View reviewed changes

sifuluo added 3 commits February 12, 2018 23:06

Update run_JRA_cfg.py

d30f828

reversed run_JRA_cfg.py to default

4e7ddc3

Merge branch 'master' of github.com:sifuluo/JetMETAnalysis

a388b43

instance with nentries <= 4 will not be shown in closure

ca0d9b0

aperloff reviewed Apr 5, 2018

View reviewed changes

updated

b7b393d

minor update ready to merge.

d7e8847

sifuluo added 3 commits August 8, 2018 03:47

compareJEC now works with commandline configurations and can generate…

eee7200

… dummyl3 files upon request.

compareJEC now works with commandline configurations and can generate…

b408bdf

… dummyl3 files upon request.

In L2Creator, ptclip is kept while ptclips and ptclipcones is added t…

eab6e92

…o specify ptclip value for specific cone sizes.

		//cout << cname << "sfsg1\tnb=" << nb << endl;
		TF1* fr = HistUtil::fit_gaussian(aux, 1.5, 1.0, 10, false);

new function of clipping when generating L2 corrections, better fit used plotting L1 offsets. #14

Are you sure you want to change the base?

new function of clipping when generating L2 corrections, better fit used plotting L1 offsets. #14

Conversation

sifuluo commented Feb 12, 2018

Choose a reason for hiding this comment

sifuluo commented Feb 13, 2018

karavdin commented Mar 16, 2018

sifuluo commented Mar 17, 2018

FHead commented Apr 5, 2018

aperloff commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aperloff Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sifuluo commented Jul 2, 2018

aperloff commented Jul 5, 2018

sifuluo commented Jul 9, 2018

aperloff commented Aug 7, 2018

aperloff Apr 9, 2018 •

edited

Loading